Skip to content

stream: drop rejection step for sync transforms - #67339

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:webstream-perf-round24
Open

mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:webstream-perf-round24

Conversation

@mcollina

@mcollina mcollina commented Nov 8, 2026 •

Copy link
Copy Markdown
Member

Round 24 of the webstreams performance work. It is independent of #66702 (no conflicts; either can land first).

Rejection step skipped for non-thenable transform results

transformStreamDefaultControllerPerformTransform chained .then(undefined, rejectionSteps) onto the result of every transform() call, mirroring the reference implementation's promiseCall(transformAlgorithm, ...).then(undefined, ...). When transform() returns a value that is not a thenable (the common transform(chunk, controller) { controller.enqueue(chunk); } shape, and the identity transform), that result cannot reject, so the rejection step can never run. The shared resolved promise is now returned directly, saving a promise and a microtask per chunk. Promise and thenable results keep the rejection step unchanged.

Observable timing

The write promise of such a transform settles one microtask earlier than in the reference implementation. The relative order of every observable stream event is unchanged: a probe of 8 transformer shapes (sync, async, returned promise, thenable, throwing, rejecting, controller.error(), controller.terminate()) at both readable high water marks, logging writes, reads, ready, close and both closed promises, produces the same event sequence on the base and patched builds; only the tick numbers of the sync, error() and terminate() shapes move. This is only observable by counting microtasks.

I also tried returning the user's promise for async transform() (with a non-rethrowing error handler registered first): it measured +0..1.6% because the async function's own promise dominates, so it is not worth the broader timing change.

Verification

  • WPT streams, compression and encoding plus the parallel webstream tests (342 files) pass.
  • Async context: the existing async-context tests pass, and a differential probe of 40 scenarios (sink write/close/abort succeeding and failing, transformer transform/flush/cancel succeeding, throwing and rejecting, parked writes at both high water marks, error()/terminate(), thenables, pipeThrough with and without an abort signal; sync and async variants) records the AsyncLocalStorage store seen by every user callback and every reaction on the returned promises. The callback → store mapping is identical to the base build in every scenario, and no callback runs outside its context.

Benchmarks

benchmark/compare.js, 20 runs, against main:

                                                                    confidence improvement accuracy (*)    (**)   (***)
webstreams/lifecycle.js kind='pipe-through' n=50000                        ***     11.78 %       ±6.06%  ±8.19% ±10.94%
webstreams/pipe-through.js kind='default' n=500000                         ***      4.86 %       ±1.89%  ±2.54%  ±3.36%
webstreams/pipe-through.js kind='transform' n=500000                       ***      3.72 %       ±1.49%  ±1.99%  ±2.63%
webstreams/encoding-streams.js len=16 kind='encode' n=100000                **      5.37 %       ±3.70%  ±4.96%  ±6.53%
webstreams/encoding-streams.js len=16 kind='decode' n=100000                **      4.99 %       ±3.07%  ±4.12%  ±5.42%
webstreams/encoding-streams.js len=1024 kind='encode' n=100000               *      2.74 %       ±2.38%  ±3.20%  ±4.21%
webstreams/lifecycle.js kind='async-iterator' n=50000                        *     -4.79 %       ±3.68%  ±4.94%  ±6.51%

The other 12 rows (creation, js_transfer, the remaining lifecycle kinds, encoding-streams len=1024 decode) are within their confidence intervals. The async-iterator row never creates a TransformStream, so the changed code does not run there; a 30-run recheck of it gives -0.96% (not significant), so it is the expected false positive for 19 comparisons.


AI Generated, reviewed by me.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams Issues and PRs related to the Web Streams API. labels Nov 8, 2026
transformStreamDefaultControllerPerformTransform chained a rejection
step onto the result of every transform() call. A non-thenable result
cannot reject, so return the shared resolved promise directly and save
a promise allocation and a microtask per chunk.

The write promise of a transform whose transform() returns a
non-thenable now settles one microtask earlier than in the reference
implementation. The relative order of all stream events is unchanged.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
mcollina force-pushed the webstream-perf-round24 branch from 3415b51 to 22e9ae8 Compare November 8, 2026 14:03
@mcollina
mcollina marked this pull request as ready for review November 11, 2026 17:48
@codecov

codecov Bot commented Nov 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.74%. Comparing base (5f602d5) to head (22e9ae8).
⚠️ Report is 61 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67339      +/-   ##
==========================================
+ Coverage   90.68%   90.74%   +0.06%     
==========================================
  Files         797      798       +1     
  Lines      279574   280101     +527     
  Branches    53778    53908     +130     
==========================================
+ Hits       253525   254175     +650     
+ Misses      16344    16210     -134     
- Partials     9705     9716      +11     
Files with missing lines Coverage Δ
lib/internal/webstreams/transformstream.js 98.81% <100.00%> (+0.01%) ⬆️

... and 79 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. web streams Issues and PRs related to the Web Streams API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants